Skip to content

deps: patch V8 to 12.4.254.16#52845

Closed
nodejs-github-bot wants to merge 2 commits intomainfrom
actions/update-v8-patch
Closed

deps: patch V8 to 12.4.254.16#52845
nodejs-github-bot wants to merge 2 commits intomainfrom
actions/update-v8-patch

Conversation

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot nodejs-github-bot commented May 5, 2024 •
edited
Loading

This is an automated patch update of V8 to 12.4.254.16.

@nodejs-github-bot nodejs-github-bot added the v8 engine Issues and PRs related to the V8 dependency. label May 5, 2024
@nodejs-github-bot
Copy link
Collaborator Author

nodejs-github-bot commented May 5, 2024

Review requested:

  • @nodejs/security-wg
  • @nodejs/v8-update

@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label May 5, 2024
@targos
Copy link
Member

targos commented May 5, 2024

@aduh95 Now I know why we didn't specify a commit message in the action settings. git node v8 already creates the commit (It's supposed to have a "Refs:" trailer).

@aduh95
Copy link
Contributor

aduh95 commented May 5, 2024

Maybe related: there's the following error in the output of the action:

Error: spawn git ENOENT
    at ChildProcess._handle.onexit (node:internal/child_process:286:19)
    at onErrorNT (node:internal/child_process:484:16)
    at process.processTicksAndRejections (node:internal/process/task_queues:82:21) {
  errno: -2,
  code: 'ENOENT',
  syscall: 'spawn git',
  path: 'git',
  spawnargs: [ 'fetch', 'origin' ]
}

https://github.com/nodejs/node/actions/runs/8958849424/job/24603576667

@nodejs-github-bot nodejs-github-bot changed the title deps: patch V8 to 12.4.254.15 deps: patch V8 to 12.4.254.16 May 12, 2024
@targos
Copy link
Member

targos commented May 12, 2024

That error was expected (I had reset the GH Actions cache).
Error is caught here: https://github.com/nodejs/node-core-utils/blob/d7b00b533dd7ef455b4f4255e4e20fd1b05b6f59/lib/update-v8/updateV8Clone.js#L17-L35
Log happens here: https://github.com/nodejs/node-core-utils/blob/d7b00b533dd7ef455b4f4255e4e20fd1b05b6f59/lib/run.js#L64

@targos
Copy link
Member

targos commented May 12, 2024

Here's a run on my fork with debug logs enabled: https://github.com/targos/node/actions/runs/9050008411/job/24864881790

Relevant:

##[debug]Changes to be committed:
##[debug]  (use "git restore --staged <file>..." to unstage)
##[debug]	modified:   deps/v8/include/v8-version.h
##[debug]	modified:   deps/v8/infra/testing/builders.pyl
##[debug]	modified:   deps/v8/src/objects/js-objects.cc
##[debug]
##[debug]Local changes found

So, I think there are two bugs:

  • We need to setup the git user/email before running the git-node command.
  • The git-node command doesn't error/fail when git commit fails.

@targos
Copy link
Member

targos commented May 12, 2024

With proper git setup: https://github.com/targos/node/actions/runs/9050110263/job/24865219130

For some reason, the action doesn't consider the new commit as a local change:

##[debug]DEFAULT_BRANCH: main
##[debug]$ git status 
##[debug]On branch test-v8-patch
##[debug]Your branch is ahead of 'origin/test-v8-patch' by 1 commit.
##[debug]  (use "git push" to publish your local commits)
##[debug]
##[debug]nothing to commit, working tree clean
No local changes
##[debug]Node Action run completed with exit code 0
##[debug]Set output result = unchanged
##[debug]Finishing: Run gr2m/create-or-update-pull-request-action@86ec1766034c81[73](https://github.com/targos/node/actions/runs/9050110263/job/24865219130#step:8:74)518f61d2075cc2a173fb8c97

@aduh95
Copy link
Contributor

aduh95 commented May 12, 2024

Maybe we should just use git push and then use the GH API to create the PR if it doesn't exist.

@targos
Copy link
Member

targos commented May 12, 2024

I'll try https://github.com/peter-evans/create-pull-request on my fork. It seems more maintained.

@targos targos mentioned this pull request May 12, 2024
Merged
@targos targos closed this May 14, 2024
@targos targos deleted the actions/update-v8-patch branch May 14, 2024 09:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

needs-ci PRs that need a full CI run. v8 engine Issues and PRs related to the V8 dependency.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@nodejs-github-bot @targos @aduh95